add test to reproduce #137687 and fix it by converting `#[crate_name]` to a new-style attribute parser #137729

jdonszelmann · 2025-02-27T13:57:10Z

compiler/rustc_driver_impl/src/lib.rs

fmease · 2025-03-06T09:02:56Z

compiler/rustc_attr_data_structures/src/attributes.rs

+        name: Symbol,
+        name_span: Span,


This could theoretically use Spanned but it doesn't necessarily lead to more readable code (and I don't remember if it's compatible with translatable diagnostics).

Suggested change

name: Symbol,

name_span: Span,

name: Symbol,

name_span: Span,

fmease · 2025-03-06T09:07:15Z

compiler/rustc_attr_parsing/src/attributes/crate_name.rs

+pub(crate) struct CratenameParser;
+
+impl SingleAttributeParser for CratenameParser {
+    const PATH: &'static [Symbol] = &[sym::crate_name];


FYI: You generally no longer need to explicitly state 'static in the ty of assoc consts.

fmease · 2025-03-06T09:12:17Z

compiler/rustc_attr_parsing/src/attributes/crate_name.rs

+
+    fn on_duplicate(cx: &AcceptContext<'_>, first_span: Span) {
+        // FIXME(jdonszelmann): better duplicate reporting (WIP)
+        cx.emit_err(UnusedMultiple {


Duplicates are warnings on master, right? So this is a breaking change?

jdonszelmann · 2025-03-06T16:12:47Z

@fmease and I decided to delay this PR for a bit until more diagnostic infra for attrs lands (hopefully next week) to make the duplicate error a warning to be consistent with current master.

fmease · 2025-03-06T16:14:30Z

r=fmease,bjorn3 once that PR lands and you can downgrade the error to a lint warning again.

@bors delegate+

bors · 2025-03-06T16:14:34Z

✌️ @jdonszelmann, you can now approve this pull request!

If @fmease told you to "r=me" after making some further change, please make that change, then do @bors r=@fmease

bors · 2025-03-13T05:36:17Z

☔ The latest upstream changes (presumably #138416) made this pull request unmergeable. Please resolve the merge conflicts.

wesleywiser · 2025-04-24T14:46:44Z

Hi @jdonszelmann and @fmease, #137687 has reached beta so it would be great if we could resolve that via a beta-backport of this PR. Do you think it's possible to resolve the review feedback and merge this PR in the next week or so?

I can probably pitch in to help if necessary 🙂

jdonszelmann · 2025-04-24T14:50:11Z

Hi there. We wanted to delay this until attribute diagnostics. Something that should not be a lot of work but I have not worked as much on recently as I maybe should. I can't to this tomorrow, but likely next week. The pr has been reviewed but there was a bit of an issue around incremental, which I now know how to solve but just haven't yet. Is that a reasonable timeframe? I am pretty busy with organising rust week atm, but knowing the solution I guess the only important thing is that it gets quick reviews? Let me know how that sounds

wesleywiser · 2025-04-24T14:53:12Z

Just to clarify, will this PR then depend on the attribute diagnostics work? (Is that #138164?)

Generally, for backports, we try to keep the backport as small as possible so if this PR can be made to work independently of that PR (possibly even with a degraded diagnostics experience), that would be highly preferred.

jdonszelmann · 2025-04-24T16:28:00Z

No. We decided to delay this because it's relatively low prio and any nice and permanent solution for this depends on that. I'll make a temp fix that at least doesn't ice separately then, that sounds better for now. I don't mind doing it, I know the problem now and then I also know how to fix it permanently later. You'll hear from me

theemathas · 2025-05-01T14:19:57Z

Note that #137687 actually has two separate problems/subissues:

A problem (lower priority) involving the #[crate_name] attribute.
A problem (higher priority, found in a crater run) involving user-defined macro_rules and derive macros.

This PR only appears to address the first problem. The second problem would probably need to have a separate fix.

jdonszelmann · 2025-05-02T13:23:01Z

ok this is weird, I've copied just the tests over to master to fix it in a temporary way, but can no longer reporoduce the issue. Something else must have solved it in the meantime. Let me see what.

jieyouxu · 2025-05-03T09:36:54Z

(I know I've said this in the other PR too, but just in case)

Just a heads up: I split #137687 into two issues:

ICE: expr in place where literal is expected (builtin attr parsing) #137687 is used to track Matthias's fuzzed regression (the #[crate_name = concat!("Cloneb")] one).
regression: ICE: expr in place where literal is expected (builtin attr parsing) #140612 is used to track the crater regressions (known to affect user macros)

jdonszelmann · 2025-08-11T12:52:44Z

@fmease finally got around to this one. The 2nd commit introduces lints for early-parsed attributes which was something that was unsupported earlier simply because no attribute needed it until now. The warning/error is unfortunately emitted twice now which is hard to fix in this PR. We simply call a very long chain of functions twice, and expect the first time to emit errors and the 2nd not. It's hard to distinguish these cases, and in some cases when we try to get metadata from rustc we only actually trigger the chain once so it's hard to even define in which call to emit errors. I left a todo for it.

jdonszelmann · 2025-08-11T12:52:57Z

should merge after #145243 which is a PR that does some generic diagnostics work that impacts this PR a lot.

fmease

I have some nits and questions but generally 👍, thanks!

fmease · 2025-08-11T15:02:50Z

compiler/rustc_hir/src/attrs/encode_cross_crate.rs

    pub fn encode_cross_crate(&self) -> EncodeCrossCrate {
        use AttributeKind::*;
        use EncodeCrossCrate::*;

        match self {
-            // tidy-alphabetical-start


Why remove tidy-alphabetical-* and the "Needed for rustdoc" comments?

fmease · 2025-08-11T15:05:28Z

compiler/rustc_interface/src/passes.rs

@@ -1125,6 +1127,28 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
    }
 }

+pub(crate) fn parse_crate_name(


Could you maybe move it below the fn get_crate_name? It feels a bit misplaced between fn run_required_analyses and fn analysis.

compiler/rustc_interface/src/util.rs

fmease · 2025-08-11T16:42:24Z

compiler/rustc_lint/src/early/diagnostics.rs

@@ -468,5 +470,15 @@ pub fn decorate_builtin_lint(
        BuiltinLintDiag::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by } => {
            lints::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by }.decorate_lint(diag)
        }
+        BuiltinLintDiag::AttributeLint(attr) => {
+            emit_attribute_lint_kind(&attr, DiagEmitter(diag));


The word emit here is misleading I feel like, this function is effectively not emitting, it's only decorating. Welp, to be fair, fns like emit_node_span_lint also don't emit necessarily (depending on fn lint_level's decision).

Er, but I see that it's like that because emit_attribute_lint_kind / SimpleLintEmitter is there to abstract over / generalize emit_node_span_lint and decorate_lint. Not sure what else to call all of these "LitEmitter"-related items tho atm 🤷

fmease · 2025-08-11T16:52:01Z

tests/ui/attributes/malformed-attrs.stderr

+  --> $DIR/malformed-attrs.rs:71:1
+   |
+LL | #[crate_name]
+   | ^^^^^^^^^^^^^ help: must be of the form: `#[crate_name = "name"]`


(will be must be of the form: `#![crate_name = "name"]` (extra !) after #145238 (future conflict))

fmease · 2025-08-11T16:58:18Z

tests/ui/invalid-compile-flags/print-crate-name-request-malformed-crate-name.stdout

@@ -0,0 +1 @@
+print_crate_name_request_malformed_crate_name


Ah, interesting ^^. This'll lead to some interesting output in the terminal where stdout & stderr are typically merged.

Ideally we shouldn't emit this (wrong) crate name … but it's probably unavoidable rn since the new attr parsing APIs don't return a Result<_, _>?

fmease · 2025-08-11T17:03:12Z

compiler/rustc_attr_parsing/src/context.rs

        };
        let mut parsed = p.parse_attribute_list(
            attrs,
            target_span,
            target_node_id,
            OmitDoc::Skip,
            std::convert::identity,
-            |_lint| {
-                panic!("can't emit lints here for now (nothing uses this atm)");
+            |AttributeLint { id, span, kind }| {


(Since I lack a bigger picture of the new attr parsing APIs) Is it desirable that parse_limited{,_should_emit} default to lint buffering by default? Shouldn't the caller decide that (via a callback)?

For example, what happens if someone calls one of these functions in a later compiler stage when all buffered lints "have been emitted already"? Can that even happen or are buffered lints only emitted when the session is destroyed (I don't know much about this logic of buffer_lint)?

fmease · 2025-08-11T17:09:32Z

compiler/rustc_middle/src/ty/context.rs

@@ -1417,15 +1418,24 @@ pub struct TyCtxt<'tcx> {
    gcx: &'tcx GlobalCtxt<'tcx>,
 }

+pub struct Simple<'tcx>(TyCtxt<'tcx>, &'static Lint, HirId, MultiSpan);


"Simple" and "SimpleLintEmitter" are quite nondescript and confusing to be totally honest. Could you rename this maybe? Not entirely sure what to call of these "things". Maybe Simple → LintEmission?? hmm >.<

(cc the naming problem of the emit_* & decorate_* abstraction I mentioned in an earlier review comment)

rustbot assigned fmease Feb 27, 2025

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 27, 2025

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the fix-137687 branch from 04bafec to 3898182 Compare February 27, 2025 14:07

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the fix-137687 branch from 3898182 to 81fc9d1 Compare February 27, 2025 14:39

bjorn3 reviewed Feb 27, 2025

View reviewed changes

compiler/rustc_driver_impl/src/lib.rs Outdated Show resolved Hide resolved

jdonszelmann force-pushed the fix-137687 branch 2 times, most recently from 4e90cee to 21838d5 Compare February 27, 2025 16:51

bjorn3 reviewed Feb 27, 2025

View reviewed changes

compiler/rustc_driver_impl/src/lib.rs Outdated Show resolved Hide resolved

jdonszelmann force-pushed the fix-137687 branch from 21838d5 to d1ee9c4 Compare February 27, 2025 17:44

fmease approved these changes Mar 6, 2025

View reviewed changes

fmease added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 6, 2025

fmease mentioned this pull request May 2, 2025

add test to reproduce #137687 and add a hotfix #140584

Closed

jdonszelmann mentioned this pull request Jun 13, 2025

Tracking issue: Attribute refactor #131229

Open

9 tasks

take attr style into account in diagnostics

65316eb

Port crate name to the new attribute system

1f5c49f

jdonszelmann force-pushed the fix-137687 branch from d1ee9c4 to 1f5c49f Compare August 11, 2025 12:50

fmease approved these changes Aug 11, 2025

View reviewed changes

fmease reviewed Aug 11, 2025

View reviewed changes

		@@ -0,0 +1 @@
		print_crate_name_request_malformed_crate_name

add test to reproduce #137687 and fix it by converting #[crate_name] to a new-style attribute parser #137729

Are you sure you want to change the base?

add test to reproduce #137687 and fix it by converting #[crate_name] to a new-style attribute parser #137729

Conversation

jdonszelmann commented Feb 27, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

fmease Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

fmease Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

fmease Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

jdonszelmann commented Mar 6, 2025

Uh oh!

fmease commented Mar 6, 2025

Uh oh!

bors commented Mar 6, 2025

Uh oh!

bors commented Mar 13, 2025

Uh oh!

wesleywiser commented Apr 24, 2025

Uh oh!

jdonszelmann commented Apr 24, 2025

Uh oh!

wesleywiser commented Apr 24, 2025

Uh oh!

jdonszelmann commented Apr 24, 2025

Uh oh!

theemathas commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdonszelmann commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jieyouxu commented May 3, 2025

Uh oh!

jdonszelmann commented Aug 11, 2025

Uh oh!

jdonszelmann commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fmease left a comment

Choose a reason for hiding this comment

Uh oh!

fmease Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

fmease Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fmease Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

fmease Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

fmease Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

add test to reproduce #137687 and fix it by converting `#[crate_name]` to a new-style attribute parser #137729

add test to reproduce #137687 and fix it by converting `#[crate_name]` to a new-style attribute parser #137729

theemathas commented May 1, 2025 •

edited

Loading

jdonszelmann commented May 2, 2025 •

edited

Loading

jdonszelmann commented Aug 11, 2025 •

edited

Loading

fmease Aug 11, 2025 •

edited

Loading

fmease Aug 11, 2025 •

edited

Loading

fmease Aug 11, 2025 •

edited

Loading